Create GitHub Action script to prevent API v1 deployment if v1 and v2 model versions don't align#2534
Conversation
anth-volk
force-pushed
the
feat/check-versions-in-gh-script
branch
from
5edef8e to
1136b66
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2534 +/- ##
==========================================
- Coverage 63.62% 63.56% -0.06%
==========================================
Files 56 56
Lines 2161 2163 +2
Branches 287 287
==========================================
Hits 1375 1375
- Misses 729 731 +2
Partials 57 57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mikesmit
left a comment
mikesmit
left a comment
There was a problem hiding this comment.
I think we should go ahead and start using this, but see comments below.
| @@ -85,9 +79,9 @@ while [ $# -gt 0 ]; do | |||
| done | |||
|
|
|||
| # Validate required arguments | |||
| if [ -z "$BUCKET_NAME" ] || [ -z "$US_VERSION" ] || [ -z "$UK_VERSION" ]; then | |||
There was a problem hiding this comment.
Suggesiton, please don't block - this is my fault, but we should configure the workflow with the bucket as an environment variable instead of an input and then set that env variable in terraform as part of deploying it.
Making a v1 deployment script hardcodes an internal detail like the bucket we store our metadata in (I.e. now it knows there is a bucket and which one) is just a whole lot of coupling that doesn't need to exist.
There was a problem hiding this comment.
This makes a lot of sense. I think you may have been reviewing this at an earlier point, as I modified the code to not even ask for a bucket name, which I suppose is even further away from what we want. Being that we don't use Terraform to deploy API v1 at the moment, I'd prefer to punt this issue, but noted - in API v2-related scripts like this, I will set via Terraform as part of deploy.
| from policyengine_api.constants import COUNTRY_PACKAGE_VERSIONS | ||
|
|
||
|
|
||
| def find_api_model_versions_and_output_to_github(): |
There was a problem hiding this comment.
question This works because we install everything for v1 globally in the build host? (i.e. so when we run it as an action it's pulling the same version of policyengine_api.constants that is in the docker container)
There was a problem hiding this comment.
Yes. This is done via the install step added before find-api-model-versions.py is invoked.
|
Waiting to merge until June 4 to enable continued regression testing. |
3 participants
Fixes #2533.
This code adds a GitHub Actions check to ensure that the full API and simulation API are running the same model versions using the simulation API's
wait_for_country_packagesCloud workflow. If both packages are using two different models, deployment fails.This was tested by running it in this PR as part of
pr.yaml, merely passing certain expected values.